-
Notifications
You must be signed in to change notification settings - Fork 3
feat: show count of results in search result view #751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe PR changes search rendering flow: Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
7016898 to
83d4686
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| current_user=request.user, search_filters=copy(search_filters) | ||
| current_user=request.user, | ||
| search_filters=copy(search_filters), | ||
| result_count=len(qs), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Misleading Result Count Shows Limited Results
The result_count displays the count after the 25-item limit is applied rather than the total matching results. Since get_filtered_published_runs returns qs[:25], calling len(qs) evaluates only those 25 items. This means the UI will show "25 results" even when hundreds of workflows match the search filters, making the count misleading for users.
| current_user=request.user, search_filters=copy(search_filters) | ||
| current_user=request.user, | ||
| search_filters=copy(search_filters), | ||
| result_count=len(qs), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Unnecessary Database Queries Waste Resources
The database query executes unconditionally even when search_filters evaluates to False (empty filters). In this case, the query results from get_filtered_published_runs and len(qs) are never used since render_search_results is only called when search_filters is truthy (line 84). This wastes a database round-trip on every explore page visit without active filters.
| if search_filters: | ||
| with gui.div(className="my-4"): | ||
| render_search_results(request.user, search_filters) | ||
| render_search_results(qs, request.user, search_filters) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Duplicate Database Queries
The database query executes twice when search filters are active: first via len(qs) on line 76 to get the count, then again on line 390 of workflow_search.py when select_related creates a new QuerySet that re-executes the query with joins. This double-execution could be avoided by computing the count differently or restructuring the flow to use a single query.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
widgets/workflow_search.py (1)
387-389: Consider removing the unuseduserparameter.The signature change to accept a pre-filtered
QuerySet[PublishedRun]is correct and aligns with the new data flow. However, theuserparameter is not used anywhere in the function body. Since filtering now happens externally, this parameter appears unnecessary.If the
userparameter is not needed for backward compatibility or planned future use, consider removing it:def render_search_results( - qs: QuerySet[PublishedRun], user: AppUser | None, search_filters: SearchFilters + qs: QuerySet[PublishedRun], search_filters: SearchFilters ):Then update the call site in
widgets/explore.py:- render_search_results(qs, request.user, search_filters) + render_search_results(qs, search_filters)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
widgets/explore.py(3 hunks)widgets/workflow_search.py(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: nikochiko
Repo: GooeyAI/gooey-server PR: 792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:23:39.522Z
Learning: In `widgets/workflow_search.py`, the `render_search_results()` function QuerySet at line 375 needs `.select_related("workspace", "saved_run", "last_edited_by").prefetch_related("tags", "versions")` to avoid N+1 queries when rendering search results with published run previews. Note: `created_by` should not be included as it's not used in workflow rendering.
📚 Learning: 2025-09-24T07:23:39.522Z
Learnt from: nikochiko
Repo: GooeyAI/gooey-server PR: 792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:23:39.522Z
Learning: In `widgets/workflow_search.py`, the `render_search_results()` function QuerySet at line 375 needs `.select_related("workspace", "saved_run", "last_edited_by").prefetch_related("tags", "versions")` to avoid N+1 queries when rendering search results with published run previews. Note: `created_by` should not be included as it's not used in workflow rendering.
Applied to files:
widgets/workflow_search.pywidgets/explore.py
📚 Learning: 2025-09-24T07:06:56.185Z
Learnt from: nikochiko
Repo: GooeyAI/gooey-server PR: 792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:06:56.185Z
Learning: In `daras_ai_v2/base.py`, the `_examples_tab()` method QuerySet at line 2164 needs `.select_related('saved_run', 'workspace', 'last_edited_by').prefetch_related('tags', 'versions')` to optimize database queries for published run rendering.
Applied to files:
widgets/explore.py
📚 Learning: 2025-09-24T07:06:56.185Z
Learnt from: nikochiko
Repo: GooeyAI/gooey-server PR: 792
File: widgets/saved_workflow.py:0-0
Timestamp: 2025-09-24T07:06:56.185Z
Learning: In `daras_ai_v2/base.py`, the `_saved_tab()` method QuerySet at line 2199 needs `.select_related('saved_run', 'workspace', 'last_edited_by').prefetch_related('tags', 'versions')` to avoid N+1 queries when rendering published runs.
Applied to files:
widgets/explore.py
🧬 Code graph analysis (2)
widgets/workflow_search.py (2)
app_users/models.py (1)
AppUser(90-262)bots/models/published_run.py (1)
PublishedRun(118-346)
widgets/explore.py (1)
widgets/workflow_search.py (3)
get_filtered_published_runs(413-420)render_search_filters(82-170)render_search_results(387-410)
🪛 Ruff (0.14.4)
widgets/workflow_search.py
388-388: Unused function argument: user
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (6)
widgets/explore.py (3)
16-16: LGTM!The import of
get_filtered_published_runsis appropriate for the new data flow.
74-77: Good use ofcopy()to prevent mutation.Using
copy(search_filters)prevents unintended side effects from modifications insiderender_search_filters.
86-86: LGTM!Correctly passes the pre-filtered queryset to
render_search_results, matching the updated function signature.widgets/workflow_search.py (3)
14-14: LGTM!The
ngettextimport enables proper pluralization for the result count display ("result" vs "results").
82-85: LGTM!The optional
result_countparameter maintains backward compatibility while enabling the new result count display feature.
130-134: LGTM!The result count caption is well-implemented:
- Properly guarded with
result_count is not None- Uses
ngettextfor correct pluralization- Hides on small screens for better mobile UX
| ) | ||
|
|
||
| search_filters = search_filters or SearchFilters() | ||
| qs = get_filtered_published_runs(request.user, search_filters) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Result count is capped at 25 instead of showing total matches.
The result count will always show at most 25 because get_filtered_published_runs returns a sliced queryset (qs[:25]). When len(qs) is called on Line 76, it evaluates the sliced queryset and returns the number of items in the slice, not the total count of matching results.
For example, if 100 workflows match the filters, the UI will incorrectly display "25 results" instead of "100 results."
To fix this, obtain the count before slicing:
search_filters = search_filters or SearchFilters()
- qs = get_filtered_published_runs(request.user, search_filters)
+ # Get filtered queryset without slicing to obtain accurate count
+ from widgets.workflow_search import build_search_filter, build_workflow_access_filter, build_sort_filter
+ from bots.models import PublishedRun
+
+ qs = PublishedRun.objects.all()
+ qs = build_search_filter(qs, search_filters, user=request.user)
+ qs = build_workflow_access_filter(qs, request.user)
+ qs = build_sort_filter(qs, search_filters)
+ result_count = qs.count()
+ qs = qs[:25]
+
render_search_bar_with_redirect(
request=request,
search_filters=search_filters,
max_width="600px",
)
with gui.div(className="mt-3"):
new_filters = render_search_filters(
current_user=request.user,
search_filters=copy(search_filters),
- result_count=len(qs),
+ result_count=result_count,
)Alternatively, refactor get_filtered_published_runs to return both the count and the sliced queryset, or accept a parameter to control whether to apply the slice.
Also applies to: 76-76
🤖 Prompt for AI Agents
In widgets/explore.py around line 66, get_filtered_published_runs currently
returns a sliced queryset (qs[:25]) so calling len(qs) later yields the slice
length (<=25) instead of the total matches; fix by obtaining the total count
before slicing (e.g., call full_qs.count() or have get_filtered_published_runs
return both total_count and sliced_qs) and then use the sliced queryset for
display while showing the pre-slice count for the results summary;
alternatively, change get_filtered_published_runs to accept a flag to disable
slicing and call it twice (once for count, once for the page) or refactor it to
return (total_count, page_qs).
Q/A checklist
How to check import time?
You can visualize this using tuna:
To measure import time for a specific library:
To reduce import times, import libraries that take a long time inside the functions that use them instead of at the top of the file:
Legal Boilerplate
Look, I get it. The entity doing business as “Gooey.AI” and/or “Dara.network” was incorporated in the State of Delaware in 2020 as Dara Network Inc. and is gonna need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Dara Network Inc can use, modify, copy, and redistribute my contributions, under its choice of terms.
Note
Shows a localized result count in the search filters and refactors result rendering to use a pre-filtered queryset computed once.
render_search_filtersnow acceptsresult_countand displays a localized count usingngettexton medium+ screens.explore.rendercomputesqs = get_filtered_published_runs(...)once and passes it to both the filters (for count) andrender_search_results.render_search_resultsnow takes a pre-filteredqsinstead of building it internally.get_filtered_published_runsandngettext; FastAPIRequestimport moved undertyping.TYPE_CHECKING.Written by Cursor Bugbot for commit 83d4686. This will update automatically on new commits. Configure here.